-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Update TypeMap attribute handling #120477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Mark TypeMapAssemblyTargetAttributes when a typemapuniverse is marked - Recurse through TypeMapAssemblyTarget assemblies to find all TypeMapAttributes - Refactor TypeMapHandler construction and initialization - Use TypeReferenceEqualityComparer for Dictionaries - Add more test coverage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates TypeMap attribute handling in the IL linker to fix issues with assembly preservation and attribute marking. The changes improve how TypeMapAttributes are handled across assemblies and ensure assemblies containing only TypeMap attributes are properly preserved.
Key changes:
- Enhanced TypeMapHandler to recursively process TypeMapAssemblyTarget assemblies
- Improved attribute marking logic to mark assemblies when TypeMap attributes need to be preserved
- Added comprehensive test coverage for cross-assembly TypeMap scenarios
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeMap.cs | Extended test case with cross-assembly TypeMap scenarios and dependency validation |
src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/Dependencies/TypeMapSecondOrderReference.cs | New test dependency assembly with TypeMap attributes for second-order reference testing |
src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/Dependencies/TypeMapReferencedAssembly.cs | New test dependency assembly with TypeMap attributes for primary reference testing |
src/tools/illink/src/linker/Linker/TypeReferenceEqualityComparer.cs | Added obsolete marker for Default property to prevent misuse |
src/tools/illink/src/linker/Linker/TypeMapHandler.cs | Major refactor of TypeMapHandler with improved initialization, recursive assembly processing, and proper assembly marking |
src/tools/illink/src/linker/Linker.Steps/SweepStep.cs | Updated IsMarkedAssembly to check both assembly and main module marking |
src/tools/illink/src/linker/Linker.Steps/MarkStep.cs | Simplified TypeMapHandler initialization and made MarkAssembly method public |
src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeMap.cs
Outdated
Show resolved
Hide resolved
Tagging subscribers to this area: @dotnet/illink |
@jkoritzinsky does this need to be backported to .NET 10? As in, I assume this bug would break CsWinRT 3.0 too? 🤔 |
…ap.cs Co-authored-by: Copilot <[email protected]>
src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeMap.cs
Outdated
Show resolved
Hide resolved
Yes, we should get this backported into .NET 10 for the first servicing release. |
…ntime into ILLinkTypeMapTests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
@jtschuster Let's make sure we a backport started when the .NET 10 staging branch is open. @Sergio0694 If this is something your team needs, please remind us in early November so it doesn't get missed. |
Fixes #120394